Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adding JUnit support to chart verifier report #432

Closed
wants to merge 2 commits into from
Closed

Adding JUnit support to chart verifier report #432

wants to merge 2 commits into from
+168 −6

Conversation

dcurran90
Copy link
Contributor

Adds JUnit output support. Example output:

<?xml version="1.0" encoding="UTF-8"?>
 <testsuites name="chart-verifier test run" tests="12" failures="5" skipped="1" timestamp="2024-04-15T11:00:44-05:00">
   <properties>
     <property name="config">
       <ApiVersion>v1</ApiVersion>
       <Kind>verify-report</Kind>
       <reportOptions></reportOptions>
       <Metadata>
         <ToolMetadata>
           <Version>1.13.3</Version>
           <Profile>
             <VendorType>partner</VendorType>
             <Version>v1.2</Version>
           </Profile>
           <ReportDigest>uint64:6719283521726907025</ReportDigest>
           <ChartUri>../helm-tester/charts/hello-world</ChartUri>
           <Digests>
             <Chart>sha256:472ca4bd30d0ac4af65d4107a2e73bc4593ea8271476e110d4b71834348c5478</Chart>
             <Package></Package>
             <PublicKey></PublicKey>
           </Digests>
           <LastCertifiedTimestamp>2024-04-15T11:00:44.838016-05:00</LastCertifiedTimestamp>
           <CertifiedOpenShiftVersions></CertifiedOpenShiftVersions>
           <TestedOpenShiftVersion>N/A</TestedOpenShiftVersion>
           <SupportedOpenShiftVersions>N/A</SupportedOpenShiftVersions>
           <ProviderDelivery>false</ProviderDelivery>
           <WebCatalogOnly>false</WebCatalogOnly>
         </ToolMetadata>
         <Overrides></Overrides>
         <ChartData>
           <Name>hello-world</Name>
           <Home></Home>
           <Version>0.1.0</Version>
           <Description>A Helm chart for Kubernetes</Description>
           <Icon></Icon>
           <APIVersion>v2</APIVersion>
           <Condition></Condition>
           <Tags></Tags>
           <AppVersion>1.16.0</AppVersion>
           <Deprecated>false</Deprecated>
           <KubeVersion></KubeVersion>
           <Type>application</Type>
           <Sources></Sources>
           <Keywords></Keywords>
           <Annotations></Annotations>
           <Maintainers></Maintainers>
           <Dependencies></Dependencies>
         </ChartData>
       </Metadata>
     </property>
   </properties>
   <testsuite name="chart-verifier test run" tests="12" failures="5" skipped="1" timestamp="2024-04-15T11:00:44-05:00">
     <testcase name="v1.1/has-kubeversion" classname="Kubernetes version is not specified" assertions="1">
       <system-out>Kubernetes version is not specified</system-out>
     </testcase>
     <testcase name="v1.0/has-readme" classname="Chart has a README" assertions="1">
       <system-out>Chart has a README</system-out>
     </testcase>
     <testcase name="v1.0/not-contains-crds" classname="Chart does not contain CRDs" assertions="1">
       <system-out>Chart does not contain CRDs</system-out>
     </testcase>
     <testcase name="v1.0/is-helm-v3" classname="API version is V2, used in Helm 3" assertions="1">
       <system-out>API version is V2, used in Helm 3</system-out>
     </testcase>
     <testcase name="v1.0/contains-test" classname="Chart test files do not exist" assertions="1">
       <system-out>Chart test files do not exist</system-out>
     </testcase>
     <testcase name="v1.0/contains-values-schema" classname="Values schema file does not exist" assertions="1">
       <system-out>Values schema file does not exist</system-out>
     </testcase>
     <testcase name="v1.0/contains-values" classname="Values file exist" assertions="1">
       <system-out>Values file exist</system-out>
     </testcase>
     <testcase name="v1.0/required-annotations-present" classname="Missing required annotations: [charts.openshift.io/name]" assertions="1">
       <system-out>Missing required annotations: [charts.openshift.io/name]</system-out>
     </testcase>
     <testcase name="v1.0/chart-testing" classname="chart Install failure: Kubernetes cluster unreachable: the server has asked for the client to provide credentials" assertions="1">
       <system-out>chart Install failure: Kubernetes cluster unreachable: the server has asked for the client to provide credentials</system-out>
     </testcase>
     <testcase name="v1.0/helm-lint" classname="Helm lint successful" assertions="1">
       <system-out>Helm lint successful</system-out>
     </testcase>
     <testcase name="v1.0/signature-is-valid" classname="Chart is not signed : Signature verification not required" assertions="1">
       <system-out>Chart is not signed : Signature verification not required</system-out>
     </testcase>
     <testcase name="v1.0/not-contain-csi-objects" classname="CSI objects do not exist" assertions="1">
       <system-out>CSI objects do not exist</system-out>
     </testcase>
   </testsuite>
 </testsuites>

Closes #232

github-advanced-security[bot]
github-advanced-security bot found potential problems Apr 15, 2024
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gosec found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

komish
komish requested changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@komish komish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this @dcurran90!

There are a couple of things I'd like to see changed here.

(1)

The JUnitXML output format should be a supplemental output format instead of being a valid value to the --output flag. The reason for this is because we will not accept junitxml reports as a part of Certification.

What I would prefer to see here is something like this (the flag's name is not prescriptive, it's just an example):

chart-verifier verify --write-junit-to /this/path/report-junit.xml <params> <chart>

This tells chart-verifier that it needs to write a junit xml representation of the test case outcomes also, in addition to writing the normal format to whatever output was specified.

Alternatively, you can choose the file for the user, and write it to the chartverifier/ directory. I kind of prefer the former, but either will work.

The reason we have to do it this way is because chart-verifier, by default, writes to STDOUT as mentioned here.

(2)

I don't necessarily believe the JUnitXML needs to include all of the metadata from the report, especially if it's supplemental. Similar to preflight, I would just focus on writing the test case definitions, and skipping properties and other metadata that aren't immediately relevant.

(3)

I think we can simplify the calls we make here from the xml library by using structs and tags. You may even be able to borrow and re-work some of those linked.

(4)

If it's possible (and I believe it should be, at least, at first glance), let's make this code internal to chart-verifier.

@dcurran90
Copy link
Contributor Author

thanks for the feedback @komish
Looking into those changes now

Moving JUnit support to secondary flag rather than main output. Using…
ea2b3b9 
… structs to reorganize results and remove unnecessary metadata from Junit output
@dcurran90 dcurran90 marked this pull request as draft May 1, 2024 10:45
@komish komish force-pushed the JUnit_Support branch 4 times, most recently from fec18ea to ea2b3b9 Compare June 11, 2024 16:39
@komish
Copy link
Contributor

komish commented Jun 11, 2024

@dcurran90

  • I've rebase this for you and resolved the file conflict in the proecss.
  • You probably need to remove your report test.
  • Anything else you might need to do before this comes out of "draft" status

@komish komish mentioned this pull request Aug 27, 2024
Open
@komish
Copy link
Contributor

komish commented Aug 27, 2024

Closing this in favor of #471

@komish komish closed this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@komish komish komish requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Please add support for reports in Junit format
2 participants
@dcurran90 @komish